-
Notifications
You must be signed in to change notification settings - Fork 130
Get rid of big "Run and Debug" button for R extension #10769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… to specify how to handle Run and Debug pane
|
E2E Tests 🚀 |
| "type": "ark", | ||
| "label": "R Debugger" | ||
| "label": "R Debugger", | ||
| "languages": ["r"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change here is what keeps us from prompting folks to install an (incompatible) extension. At the very minimum, we should contribute a language here to avoid that prompt.
extensions/positron-r/package.json
Outdated
| "label": "R Debugger" | ||
| "label": "R Debugger", | ||
| "languages": ["r"], | ||
| "supportsUILaunch": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we work on building Positron itself, the IDE registers the vscode://schemas/vscode-extensions schema at startup. This item is not in that schema, so if we go this direction overall, we will see a diagnostic in this file moving forward:
Property supportsUILaunch is not allowed.
I did some exploration of how to silence this diagnostic but did not find anything quickly/easily.
| export const CONTEXT_DEBUGGERS_AVAILABLE = new RawContextKey<boolean>('debuggersAvailable', false, { type: 'boolean', description: nls.localize('debuggersAvailable', "True when there is at least one debug extensions active.") }); | ||
| export const CONTEXT_DEBUG_EXTENSION_AVAILABLE = new RawContextKey<boolean>('debugExtensionAvailable', true, { type: 'boolean', description: nls.localize('debugExtensionsAvailable', "True when there is at least one debug extension installed and enabled.") }); | ||
| // --- Start Positron --- | ||
| export const CONTEXT_DEBUGGER_SUPPORTS_UI_LAUNCH = new RawContextKey<boolean>('debuggerSupportsUILaunch', true, { type: 'boolean', description: nls.localize('debuggerSupportsUILaunch', "True when the debugger for the current file supports launching from the Run and Debug UI. Some debuggers (like R) use alternative debugging approaches.") }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new context key is what allows extensions to say, "Hey, don't turn on all the debugger buttons for this type of file."
lionel-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great in R files!
Experimenting with it, I found the experience to be a bit jittery and confusing with other editors though.
https://github.com/user-attachments/assets/c857f89f-722b-4553-bf72-90b4f45fb400
Also when no editor is opened:
Should we instead look at the currently opened console, retrieve its languageId, and use that instead of the current editor's language to determine whether to show the run and debug button?
| // if (language && this.debugService.getAdapterManager().someDebuggerInterestedInLanguage(language)) { | ||
| // Find debuggers for this language and check if any support UI launch | ||
| const adapterManager = this.debugService.getAdapterManager(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no changes to the if condition right? Probably better to leave this as is and refetch the adapterManager below, to reduce the amount of stuff in Positron fences.
| "view": "debug", | ||
| "contents": "Positron currently provides initial debugging support for R code.\n[Learn more](https://positron.posit.co/guide-r.html#debugging)", | ||
| "when": "debugStartLanguage == 'r'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! We'll be able to use this to provide a "Pause" button to interrupt a running R command and drop in the debugger. This would be ideal with a context key that indicates whether the current console is busy.
jmcphers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not test this since @lionel- covered that, but code LGTM.
Co-authored-by: Jonathan <[email protected]> Signed-off-by: Julia Silge <[email protected]>
Co-authored-by: Lionel Henry <[email protected]> Signed-off-by: Julia Silge <[email protected]>
|
Thank you for the review, @lionel-! I addressed a chunk of the jittery-ness with 54a2564, so it is now improved. However, there are still a couple of issues because of the That But then it is never cleared. 😱 I don't think there's a way for us to handle that from Positron, and this is why the big "Show automatic Python configurations" button follows you around once you get it. I'd like to propose we log that and think on how to best solve the problem. (Maybe they would take a PR?)
I do not think that would help any of this, no, because of how the debugger is so linked to the editor, and we need to offer the right debugger on, say, HTML files and other non-R, non-Python editors. |
To be clear my suggestion was to use the console language only for cases where there is no editor, or no associated language with the editor. |
lionel-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a nice improvement!
I think this could be improved by not showing the Run and Debug button when no file or language are available (or by looking at the language of the active console as suggested).
That said users are not likely to have this pane open on startup, they probably only open it when actively working with R files, in which case they'll find the improved pane 👍
|
I opened #10938 for us to consider our options with the |
Addresses #3681
This is a little heavy handed (I would have liked to find a solution here that involves a less dramatic change) but this area of upstream doesn't seem to involve a lot of churn, so I think it's likely useful for us to make this type of adjustment. Quarto files are another type where this whole pane is quite confusing, and we can use a similar approach in that extension (set
supportsUILaunchand contribute aviewsWelcomebutton or something).With this PR, you now see this in the "Run and Debug" pane for
.Rfiles:You should still see the normal content for a
.pyfile or.qmdfile or another kind of file.Release Notes
New Features
Bug Fixes
QA Notes
.Rfiles..py,.qmd, or other types of files.